Fix GH-20557: Build make -C ext/* install-modules w/o modules/* #20558
Conversation
update the build recipe template as in PHP-8.5 ext/opcache errors on
`make -C ext/opcache install` as modules/* does not resolve to actual
filenames any longer,
but to `modules/*` verbatim which fails the `install` command yielding
the following diagnostics:
cp: cannot stat 'modules/*': No such file or directory
and exit status 1 (resolved by make as build error 2).
hakre
changed the title
|
Note that in fact it might need to target PHP-8.5 branch though. |
TimWolla
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
After both reading the issue and the description of this PR, it is still not clear to me what this is supposed to fix. Installing OPcache as a separate module is no longer supported and this is documented as part of the migration guide.
If anything, it makes sense here to check whether the directory exists and contains files for installation. Otherwise, yes. That type of installation with phpize won't work in any case anymore for opcache extension since PHP-8.5. |
|
Thanks @petk, I couldn't have put it better myself. |
|
Thank you @TimWolla for your candid feedback. |
There was a problem hiding this comment.
The PR description is unchanged from my previous review and does not provide an argument or example in favor of this change that doesn't involve trying to install OPcache as a shared module. This makes it impossible to evaluate the correctness of this change.
Should this check $PHP_MODULES instead?
@petk As far as I'm concerned, “no module is available” is an error situation and emitting an error message is the correct consequence of that. |
|
Actually, yes. I'm exactly at this point now also in my CMake-based build system. :D Yes, building extensions, such as opcache, json, hash, random, date, etc. in "phpize" style should emit some error, yes. Otherwise, also this bug in Docker image wouldn't be noted. It's only that install modules target in Makefile is probably a bit inconvenient place for detecting and emitting such error, yes. Edit: Ah, I see. And there is even |
Yes, this is parenting in the fix. Btw. have you figured out in CMake to define all install files for the shared objects? Great project btw.. |
Yes. It felt almost like a rocket science. Very complex but quite neat overall. It's resolved over CMake configuration files in one of my local branches and even with upcoming CPS experimentation. :D |
If anything, then finding no actionable in the change request description for the pull request description. Otherwise, yes, obviously., voiding the invocation voids the exit status and the solution. Please share more context (What have you tried? What did happen? What did you expect?). |
4 participants
Update the build recipe template as in PHP-8.5 ext/opcache errors on
make -C ext/opcache installas modules/* does not resolve to actual filenames any longer,but to
modules/*verbatim which fails theinstallcommand yielding the following diagnostics:and exit status 1 (resolved by make as build error 2).
Important
Install is a standard target and we keep the invocation unconditional as the user could be currently remaking.
Note
ext/opcache is only exemplary, this can be any ext/* that makes use of the install-modules target to install zero modules. (that is the recipe with the fix here)